add task solution#4482
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a core requirement around navigation decomposition is not yet met and this affects both structure and functionality. Navigation is still fully implemented inside Header.jsx (<nav className="navigation"> with links), while Navigation.jsx only returns a placeholder <nav className="nav">...</nav>, so the real navigation has not actually been moved into the Navigation component as required. To fix this, move the <nav> and its links from Header.jsx into Navigation.jsx, use the same classes (navigation, navigation__link) or update styles accordingly, and have Header render only header-specific content (optionally including <Navigation /> instead of inline markup). Also, move navigation-specific styles (e.g. .navigation, .navigation__link) out of Header.css into Navigation.css so that styles are colocated with the Navigation component [CHECKLIST ITEM #2], and ensure Navigation.css is present and committed [CHECKLIST ITEM #3]. Finally, address the lint error by making sure Navigation.css is not empty (add the actual navigation rules you moved from Header.css). Once these changes are in place, your decomposition will fully match the task requirements; everything else in your component structure and use of default exports looks good and is a solid foundation to build on.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| </main> | ||
| <> | ||
| <Header /> | ||
| <Navigation /> |
There was a problem hiding this comment.
Navigation is rendered here, but in the current codebase the navigation markup is still inside Header.jsx. To satisfy the description ("Move navigation into a Navigation component"), move the <nav> with links fully into Navigation and remove the duplicate navigation from Header.
| background: white; | ||
| box-shadow: rgba(0, 0, 0, 0.05) 0 3px 15px; | ||
| } | ||
|
|
There was a problem hiding this comment.
The navigation has been extracted into a Navigation component, but the file still imports and (per the separate Header.jsx file) renders a <nav> in the header; ensure all navigation markup is consistently handled in Navigation as required by the description and that Header only handles header-specific content.
| } | ||
|
|
||
| @media (min-width: 500px) { | ||
| .article { |
There was a problem hiding this comment.
Header.css still contains .navigation__link rules; since navigation should be in its own component with its own CSS, move these rules into Navigation.css (or another navigation-specific stylesheet) so each BEM block has separate styles as required.
| vitae feugiat sem magna eget massa. Vivamus quis tincidunt dolor. Fusce | ||
| efficitur, orci non vestibulum consequat, lectus turpis bibendum odio, | ||
| in efficitur leo felis sed justo. Fusce commodo iaculis orci, quis | ||
| imperdiet urna. Sed mollis facilisis lacus non condimentum. Nunc | ||
| efficitur massa non neque elementum semper. Vestibulum lorem arcu, | ||
| tincidunt in quam et, feugiat venenatis augue. Donec sed tincidunt |
There was a problem hiding this comment.
Navigation is required to be a separate component, but Header still contains the <nav> block and link elements. This conflicts with the task requirement to move navigation into a Navigation component and with the existence of a separate Navigation.jsx file.
| tincidunt id odio id feugiat. | ||
| </p> | ||
| <p className="article__paragraph"> | ||
| In ac nisi lacus. Fusce est dolor, tincidunt ut bibendum vitae, | ||
| fermentum ac quam. Aliquam pretium tristique nibh quis iaculis. In et | ||
| cursus ex, eu aliquet ex. Proin facilisis lacus sit amet sapien | ||
| ultrices, ut vehicula arcu lobortis. Vivamus mollis ipsum ut hendrerit | ||
| molestie. Morbi lacinia, sapien eu dictum dignissim, tellus tortor | ||
| congue magna, sit amet bibendum libero nisi id massa. | ||
| </p> | ||
| <p className="article__paragraph"> | ||
| Donec arcu elit, euismod vel lobortis eu, fringilla sit amet dolor. Cras | ||
| congue, massa nec sagittis mollis, dui felis ultrices magna, tincidunt | ||
| finibus lorem quam in sem. Morbi odio turpis, pulvinar sit amet |
There was a problem hiding this comment.
.navigation__link styles belong to the navigation BEM block and should be moved into a Navigation.css file in the Navigation component folder. Keeping these styles in Header.css means CSS is not fully split per component as required by the description and checklist item #2.
| // Create a Header function returning the HTML of header block | ||
|
|
There was a problem hiding this comment.
The Navigation component currently returns a placeholder <nav className="nav">...</nav>, which doesn’t match the real navigation markup or classes used elsewhere (navigation, navigation__link). To satisfy the requirement of moving navigation into this component, copy the actual links from Header.jsx and reuse the correct classes here.
| export default function Navigation() { | ||
| return <nav className="nav">...</nav>; |
There was a problem hiding this comment.
Navigation is importing Navigation.css but only returns a placeholder <nav className="nav">...</nav>. This doesn’t match the actual navigation markup in Header and doesn’t use the existing navigation-related styles. You should move the real nav markup here and align the class names with the navigation CSS, then remove the nav from Header.
| /* Put welcome styles here */ | ||
| .welcome { | ||
| height: 100vh; | ||
| background: coral; |
There was a problem hiding this comment.
Navigation.jsx imports ./Navigation.css and returns a placeholder <nav className="nav">...</nav>. This doesn’t match the real navigation markup and likely relies on a CSS file that is missing or empty. It violates checklist item #2 (styles per component) and #3 (all files added to git) if Navigation.css is not present or committed. Update this component to render the actual links and ensure Navigation.css exists with the navigation styles.
| @@ -1,5 +1,12 @@ | |||
| // import a css file containig welcome styles | |||
There was a problem hiding this comment.
Navigation.jsx imports ./Navigation.css, but that file is not present in the submitted code. This likely violates checklist item #3 ("check if you added ALL your files to git"). Ensure Navigation.css exists and is committed, and that its styles match the classes used in this component.
| // Create a Welcome function returning the HTML of welcome block | ||
|
|
There was a problem hiding this comment.
The component currently returns a placeholder <nav className="nav">...</nav>, but the task expects the actual navigation HTML to be moved here from Header. Replace the ... with the real link structure (and appropriate class names) so navigation is fully implemented in this component.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core requirements from the task are implemented correctly: App is decomposed into Header, Welcome, Article, and Navigation, each in its own folder with colocated CSS, and all components use export default as required. The navigation markup has been properly moved out of Header into Navigation.jsx, and Header now only renders header content plus <Navigation />, which matches the decomposition and project structure expectations [CHECKLIST ITEM #2]. There are no functional issues or broken behavior evident from the current code, and none of the remaining points rise to the level of a critical blocker.
The only remaining gap is minor: Navigation.jsx uses className="navigation", but Navigation.css only defines .navigation__link rules; if you want the navigation block to be fully self-contained, you could add a .navigation rule there mirroring the original container styling. This is a structural/style refinement rather than a required fix, so it does not block acceptance. Overall, you’ve done a solid job aligning the components, CSS, and exports with the task requirements—well done.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| @@ -0,0 +1,15 @@ | |||
|
|
|||
| .navigation__link { | |||
There was a problem hiding this comment.
The Navigation component uses className="navigation", but Navigation.css only defines .navigation__link styles. To fully decompose CSS per BEM block as required, consider adding/moving the .navigation block styles into this file so the navigation container is styled alongside its links.
2 participants
DEMO LINK